-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add custom NodePort to helm controller-service. #477
Conversation
Hi @sombralibre Thanks for the PR! Can you give the version number in Chart.yaml a minor bump? |
Hi @sombralibre, Apologies for not mentioning this in my initial review. I think that the nodePort customisation should be implemented slightly differently, taking into consideration the other useful customisation options for http and https ports. A possible solution could look like: values.yaml service:
httpPort:
enable: true
port: 80
nodePort: ""
httpsPort:
enable: true
port: 443
nodePort: "" controller-service.yaml ports:
{{- if .Values.controller.service.httpPort.enable }}
- port: {{ .Values.controller.service.httpPort.port }}
targetPort: 80
. . .
{{- end }}
{{- if .Values.controller.service.httpsPort.enable }}
- port: {{ .Values.controller.service.httpsPort.port }}
targetPort: 443
. . .
{{- end }} What do you think? I do realise that this solution requires some major changes to this PR. Therefore I’d be happy to implement them. However, if you still would like to update this PR, please let me know. |
Looks cool, I want to perform these changes because they will be needed to use this ingress-controller into my company. So can I push the changes to this PR? or will be better to open a new one? |
@sombralibre |
I've pushed the changes to this new PR #479 |
Closing in favour of #479 |
Proposed changes
Add two configuration parameters to helm chart controller service, in order to use custom ports number when using NodePort.
These are the new parameters:
controller.service.NodePort.Http
controller.service.NodePort.Https
Checklist
Before creating a PR, run through this checklist and mark each as complete.